-
Notifications
You must be signed in to change notification settings - Fork 195
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
(0.95.3) Restores open boundary condition functionality #3937
base: main
Are you sure you want to change the base?
Conversation
shoudl we add a test to make sure it keeps working? |
a test is a good idea. |
@@ -61,7 +61,7 @@ end | |||
|
|||
const c = Center() | |||
|
|||
@inline function _fill_west_open_halo!(j, k, grid, ϕ, bc::FEOBC, loc, clock, model_fields) | |||
@inline function _fill_west_halo!(j, k, grid, ϕ, bc::FEOBC, loc, clock, model_fields) | |||
Δx₁ = xspacing(1, j, k, grid, c, c, c) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Δx₁ = xspacing(1, j, k, grid, c, c, c) | |
Δx₁ = Δxᶜᶜᶜ(1, j, k, grid) |
Probably better to use the pointwise operators from here (and in the rest of this file)
I might be missing something but I think that the changes in #3834 make it so that open halos are filled even when Edit: I have now added it |
# Returns the boundary conditions a specific side for `FieldBoundaryConditions` inputs and | ||
# a tuple of boundary conditions for `NTuple{N, <:FieldBoundaryConditions}` inputs | ||
for dir in (:west, :east, :south, :north, :bottom, :top) | ||
extract_side_bc = Symbol(:extract_, dir, :_bc) | ||
@eval begin | ||
@inline $extract_side_bc(bc) = bc.$dir | ||
@inline $extract_side_bc(bc) = retrieve_bc(bc.$dir) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@simone-silvestri I think we need this analogously to retrieve_open_bc
@glwagner @simone-silvestri I think this should be all done now and we should merge ASAP since open boundaries are currently broken |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sounds good!
Let's maybe try to release both this and #3606 together to avoid multiple patch releases. Both seem to be ready to merge. |
The refactoring in #3834 broke flat extrapolation open boundary conditions. This restores them.
Closes #3975